feat: add pwa app and dashboard interaction updates#735
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (55)
📝 WalkthroughWalkthroughThe PR updates route redirects and sitemap generation, refactors RAG retrieval and caching, adds Supabase cleanup and query-correction schema changes, revises form and service interfaces, and expands accessibility, mobile, regression, and tooling coverage. ChangesRouting and retrieval
Forms, services, and UI
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
CI triageCI failed on this PR. Automated classification of the 7 failed job(s):
Compared with main CI run #2833 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
659472a to
df55f5c
Compare
df55f5c to
eb5bc2d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/forms/form-detail-page.tsx (1)
33-34: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRestore the declarations removed by this refactor.
Removing
codeTextleaves its Line 231 reference unresolved, whilesourceItemsandverificationItemsare also used without declarations. Type checking will fail until the import and derived arrays are restored or their usages are removed.Also applies to: 239-273
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/forms/form-detail-page.tsx` around lines 33 - 34, Restore the codeText import and the sourceItems and verificationItems declarations in the form-detail component, ensuring they provide the existing usages around lines 231 and 239-273 with the expected derived data. Alternatively, remove those usages only if the refactor no longer requires them, while keeping the component type-checking successfully.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/generate-site-map.ts`:
- Around line 394-405: Update the route collection used by the “Main product
routes” section so product route handlers from publicRouteHandlers are included
alongside productRoutes before generating lines. Preserve their exclusion from
“Public utility route handlers,” and ensure routes such as /applications,
/differentials/presentations, and /medications appear in the main product
section.
In `@src/app/differentials/presentations/route.ts`:
- Around line 5-10: Update the query selection in GET so a missing or
blank-after-trimming query parameter falls back to q. Trim each candidate before
choosing it, while preserving the existing selectedIds parsing behavior.
In `@src/components/forms/form-detail-page.tsx`:
- Line 589: Update the label for the action opening form.source.url in the form
detail component so the visible text and accessible name accurately describe
opening the official source, replacing the misleading “Related forms” wording
while preserving the existing link behavior.
In `@src/lib/rag.ts`:
- Around line 1362-1369: Update the request-scoped RAG flows around the shown
classifier verdict await to preserve args.signal at I/O boundaries: pass the
request signal into local Supabase queries/RPCs, including alias, correction,
and retrieval operations, so cancellation stops those awaits promptly. Keep
shared classifier work independent when intentionally reused, while ensuring
aborted requests still propagate cancellation rather than being swallowed by
fallback handling.
In `@supabase/drift-manifest.json`:
- Around line 2-6: Revert the changes to the generated drift-manifest artifact,
or regenerate it using a healthy Docker-backed replay before merging. Ensure the
resulting manifest is not marked freshly generated unless its timestamp and
schema_sha256 reflect that replay, and verify it includes
documents_sync_title_words.
In `@supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql`:
- Around line 2-11: Align the document_table_facts_text_trgm_idx migration with
the trgm_matches RPC predicate by indexing only table_title, row_label, and
clinical_parameter. Remove threshold_value and action from the expression, or
remove this redundant migration if the existing narrow index already provides
the correct expression; do not retain both indexes.
In `@tests/audit-content-services-regressions.test.ts`:
- Around line 208-215: Align the source-link class assertion in the test with
the class used by the anchor in FormDetailPage. Update the min-h-tap expectation
to match the component’s existing min-h-10 class unless the component is
intentionally being changed to use the larger tap target.
---
Outside diff comments:
In `@src/components/forms/form-detail-page.tsx`:
- Around line 33-34: Restore the codeText import and the sourceItems and
verificationItems declarations in the form-detail component, ensuring they
provide the existing usages around lines 231 and 239-273 with the expected
derived data. Alternatively, remove those usages only if the refactor no longer
requires them, while keeping the component type-checking successfully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: af0a49bd-f391-482f-905d-a53a729e36d8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (51)
.npmrcdocs/codebase-index.mddocs/rag-scalability-wip-review-handover-2026-07-15.mddocs/site-map.mdpackage.jsonplaywright.config.tsscripts/check-codebase-index-coverage.mjsscripts/check-github-action-pins.mjsscripts/check-m13-migration.tsscripts/generate-site-map.tssrc/app/applications/page.tsxsrc/app/applications/route.tssrc/app/differentials/presentations/page.tsxsrc/app/differentials/presentations/route.tssrc/app/forms/[slug]/page.tsxsrc/app/layout.tsxsrc/app/medications/page.tsxsrc/app/medications/route.tssrc/app/page.tsxsrc/app/services/[slug]/page.tsxsrc/components/clinical-dashboard/answer-status.tsxsrc/components/clinical-dashboard/favourites-command-library-page.tsxsrc/components/clinical-dashboard/master-search-header.tsxsrc/components/differentials/differential-presentation-workflow-page.tsxsrc/components/differentials/differential-stream-page.tsxsrc/components/forms/form-detail-page.tsxsrc/components/forms/forms-home-page.tsxsrc/components/privacy-input-notice.tsxsrc/components/services/service-detail-page.tsxsrc/components/services/services-navigator-page.tsxsrc/lib/legacy-home-redirect.tssrc/lib/rag-candidate-sources.tssrc/lib/rag.tssrc/lib/service-navigator-metrics.tssupabase/drift-manifest.jsonsupabase/migrations/20260714180000_patch_rag_and_corrector_scalability.sqlsupabase/migrations/20260714190000_document_table_facts_trgm_idx.sqlsupabase/migrations/20260717010000_harden_rag_scalability_patch.sqlsupabase/schema.sqltests/audit-content-services-regressions.test.tstests/audit-navigation-auth-regressions.test.tstests/codebase-index-coverage.test.tstests/mobile-interaction-regressions.test.tstests/rag-chunk-load-cache.test.tstests/registry-corpus.test.tstests/retrieval-hydration-scope.test.tstests/site-map.test.tstests/supabase-schema.test.tstests/ui-accessibility.spec.tstests/ui-smoke.spec.tstests/ui-tools.spec.ts
💤 Files with no reviewable changes (4)
- src/app/differentials/presentations/page.tsx
- src/app/medications/page.tsx
- src/app/applications/page.tsx
- src/app/page.tsx
| const publicUtilityRouteHandlers = data.publicRouteHandlers.filter( | ||
| (route) => !productRouteHandlerPaths.has(route.route), | ||
| ); | ||
|
|
||
| const lines = [ | ||
| "# Clinical KB Site Map", | ||
| "", | ||
| "This file is generated by `npm run sitemap:update`. Run `npm run sitemap:check` to verify it is current.", | ||
| "", | ||
| ...section( | ||
| "Main product pages", | ||
| "Main product routes", | ||
| productRoutes.map((route) => routeLine(route, routeDescriptions)), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include product route handlers in the main product section.
These handlers are filtered out of “Public utility route handlers” but never added to productRoutes. Consequently, /applications, /differentials/presentations, and /medications are absent from “Main product routes,” failing the assertion at tests/site-map.test.ts Lines 88-94.
Proposed fix
- const productRoutes = data.pageRoutes.filter(
+ const productRoutes = [
+ ...data.pageRoutes.filter(
(route) =>
!route.route.startsWith("/api") &&
!route.route.startsWith("/mockups") &&
![
// ...
].includes(route.route),
- );
+ ),
+ ...data.publicRouteHandlers.filter((route) => productRouteHandlerPaths.has(route.route)),
+ ].sort((left, right) => left.route.localeCompare(right.route));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/generate-site-map.ts` around lines 394 - 405, Update the route
collection used by the “Main product routes” section so product route handlers
from publicRouteHandlers are included alongside productRoutes before generating
lines. Preserve their exclusion from “Public utility route handlers,” and ensure
routes such as /applications, /differentials/presentations, and /medications
appear in the main product section.
| export function GET(request: NextRequest) { | ||
| const query = (request.nextUrl.searchParams.get("query") ?? request.nextUrl.searchParams.get("q"))?.trim(); | ||
| const selectedIds = (request.nextUrl.searchParams.get("ids") ?? "") | ||
| .split(",") | ||
| .map((id) => id.trim()) | ||
| .filter(Boolean); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fall back to q when query is blank.
Nullish coalescing treats query="" or whitespace as present, so a valid q is discarded.
Proposed fix
- const query = (request.nextUrl.searchParams.get("query") ?? request.nextUrl.searchParams.get("q"))?.trim();
+ const query =
+ request.nextUrl.searchParams.get("query")?.trim() ||
+ request.nextUrl.searchParams.get("q")?.trim();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function GET(request: NextRequest) { | |
| const query = (request.nextUrl.searchParams.get("query") ?? request.nextUrl.searchParams.get("q"))?.trim(); | |
| const selectedIds = (request.nextUrl.searchParams.get("ids") ?? "") | |
| .split(",") | |
| .map((id) => id.trim()) | |
| .filter(Boolean); | |
| export function GET(request: NextRequest) { | |
| const query = | |
| request.nextUrl.searchParams.get("query")?.trim() || | |
| request.nextUrl.searchParams.get("q")?.trim(); | |
| const selectedIds = (request.nextUrl.searchParams.get("ids") ?? "") | |
| .split(",") | |
| .map((id) => id.trim()) | |
| .filter(Boolean); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/differentials/presentations/route.ts` around lines 5 - 10, Update the
query selection in GET so a missing or blank-after-trimming query parameter
falls back to q. Trim each candidate before choosing it, while preserving the
existing selectedIds parsing behavior.
| > | ||
| <FileText className="h-4 w-4" aria-hidden /> | ||
| <span>Source</span> | ||
| <span>Related forms</span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Label this source-opening action accurately.
The button says “Related forms,” but it opens form.source.url and its accessible name says “Open official source.”
Proposed fix
-<span>Related forms</span>
+<span>Official source</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span>Related forms</span> | |
| <span>Official source</span> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/forms/form-detail-page.tsx` at line 589, Update the label for
the action opening form.source.url in the form detail component so the visible
text and accessible name accurately describe opening the official source,
replacing the misleading “Related forms” wording while preserving the existing
link behavior.
| "generated_at": "2026-07-17T14:08:59.595Z", | ||
| "generator": "scripts/generate-drift-manifest.ts", | ||
| "postgres_image": "supabase/postgres:17.6.1.127", | ||
| "schema_sha256": "bde3b7c60872a8c4493555e6670c30ac777a862a12567ef2091dfc050c86240e", | ||
| "replay_seconds": 77, | ||
| "schema_sha256": "e93fe90cae38654e77b7be0f81e5a0c90b0c7db2fd37d76e32f4f52a089eff45", | ||
| "replay_seconds": 30, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not mark the stale drift snapshot as freshly generated.
The PR objective says Docker-backed regeneration is still pending, yet Lines 2–6 publish a new timestamp/hash. The snapshot also omits documents_sync_title_words, so accepting it as authoritative can hide a deployment where title correction stops updating. Revert this artifact or regenerate it from a healthy replay before merge.
Also applies to: 6327-6334
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/drift-manifest.json` around lines 2 - 6, Revert the changes to the
generated drift-manifest artifact, or regenerate it using a healthy
Docker-backed replay before merging. Ensure the resulting manifest is not marked
freshly generated unless its timestamp and schema_sha256 reflect that replay,
and verify it includes documents_sync_title_words.
| create index if not exists document_table_facts_text_trgm_idx | ||
| on public.document_table_facts using gin ( | ||
| lower( | ||
| coalesce(table_title, '') || ' ' || | ||
| coalesce(row_label, '') || ' ' || | ||
| coalesce(clinical_parameter, '') || ' ' || | ||
| coalesce(threshold_value, '') || ' ' || | ||
| coalesce(action, '') | ||
| ) extensions.gin_trgm_ops | ||
| ); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Match the trigram index expression to the RPC predicate.
trgm_matches searches only title, row label, and clinical parameter, while this index additionally includes threshold and action. PostgreSQL therefore cannot use it for that predicate, and the existing narrow index already matches. Remove this migration, or update the RPC expression and replace the narrow index rather than retaining both.
🧰 Tools
🪛 Squawk (2.59.0)
[warning] 2-11: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.
(require-concurrent-index-creation)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql` around
lines 2 - 11, Align the document_table_facts_text_trgm_idx migration with the
trgm_matches RPC predicate by indexing only table_title, row_label, and
clinical_parameter. Remove threshold_value and action from the expression, or
remove this redundant migration if the existing narrow index already provides
the correct expression; do not retain both indexes.
|
@copilot resolve the merge conflicts on this branch. |
- Resolve conflict in src/components/forms/form-detail-page.tsx: add FormCodeBadge integration and formCode helper from main, fix PathwayContextCard to use parallelForms/afterForms from main, add missing codeText import, add code prop to PathwayContextCard - Resolve conflict in src/components/services/services-navigator-page.tsx: accept main's size-tap CSS while preserving aria-pressed/comparison labels - Fix TypeScript errors from merge: add chunkLoadCache (createChunkLoadCache) to searchChunksWithTelemetry, import serviceNavigatorMetrics/canCompareServices in services-navigator-page, expand HomeProps searchParams type with q/focus/run fields, add signal parameter to callVersionedRetrievalRpc for abort propagation, add signal option to analyzeQueryWithClassifierFallback for per-caller cancellation
Head branch was pushed to by a user without write access
Merge conflicts resolved in commit Conflicts resolved:
TypeScript errors fixed post-merge:
|
Summary
Notes
Verification
npm run check:runtime(pass)npm run test:e2e:pr --project=chromium --grep-invert "@quarantine|@mockup"(pass)test:e2e:prbrowser coverage path for Chromium with quarantine/mockup filter.verify:uirun after fixups.Clinical Governance Preflight
Risk and rollout
src/app/layout.tsxand normalized@/lib-to-relative imports in affected API routes; verified by successful runtime build/test command.